Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[processor/memorylimiter] Add profiles support to memorylimiter processor #12454

Merged
merged 1 commit into from
Feb 27, 2025
Merged

[processor/memorylimiter] Add profiles support to memorylimiter processor #12454

merged 1 commit into from
Feb 27, 2025
+201 −28

Conversation

iblancasa
Copy link
Contributor

Description

Add profiles support to the memorylimiter processor.

Link to tracking issue

Fixes #12453

@iblancasa iblancasa requested a review from a team as a code owner February 21, 2025 11:07
@iblancasa iblancasa requested a review from jmacd February 21, 2025 11:07
@jmacd jmacd mentioned this pull request Feb 21, 2025
Open
@iblancasa iblancasa mentioned this pull request Feb 24, 2025
Open
@codecov Codecov
Copy link

codecov bot commented Feb 24, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.89189% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.10%. Comparing base (0d81535) to head (ec7daba).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
processor/memorylimiterprocessor/factory.go 85.71% 2 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (91.89%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #12454      +/-   ##
==========================================
- Coverage   92.11%   92.10%   -0.01%     
==========================================
  Files         467      467              
  Lines       25252    25269      +17     
==========================================
+ Hits        23260    23274      +14     
- Misses       1590     1592       +2     
- Partials      402      403       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iblancasa iblancasa force-pushed the 12453 branch 2 times, most recently from 042af5e to 7c44f5d Compare February 24, 2025 14:17
@dmathieu
Copy link
Member

You need to run make genotelcorecol on your local checkout to fix the CI failure.

@iblancasa
Copy link
Contributor Author

iblancasa commented Feb 24, 2025 •
edited
Loading

When I run make crosslink:

diff --git a/cmd/otelcorecol/go.mod b/cmd/otelcorecol/go.mod
index df4992226..e70e882b5 100644
--- a/cmd/otelcorecol/go.mod
+++ b/cmd/otelcorecol/go.mod
@@ -303,3 +303,5 @@ replace go.opentelemetry.io/collector/semconv => ../../semconv
 replace go.opentelemetry.io/collector/service => ../../service

 replace go.opentelemetry.io/collector/service/hostcapabilities => ../../service/hostcapabilities
+
+replace go.opentelemetry.io/collector/processor/processorhelper/xprocessorhelper => ../../processor/processorhelper/xprocessorhelper

But when I run make genotelcorecol:

diff --git a/cmd/otelcorecol/go.mod b/cmd/otelcorecol/go.mod
index e70e882b5..df4992226 100644
--- a/cmd/otelcorecol/go.mod
+++ b/cmd/otelcorecol/go.mod
@@ -303,5 +303,3 @@ replace go.opentelemetry.io/collector/semconv => ../../semconv
 replace go.opentelemetry.io/collector/service => ../../service

 replace go.opentelemetry.io/collector/service/hostcapabilities => ../../service/hostcapabilities
-
-replace go.opentelemetry.io/collector/processor/processorhelper/xprocessorhelper => ../../processor/processorhelper/xprocessorhelper

@dmathieu any hints?

@dmathieu
Copy link
Member

You need to add xprocessorhelper to this config here:
https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/otelcorecol/builder-config.yaml#L39

You also need to add it there:
https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/builder/internal/builder/main_test.go#L40

You should be able to run both crosslink and otelcolgen in any order and keep a clean repository.

@iblancasa
Copy link
Contributor Author

cc @open-telemetry/collector-maintainers

@dmathieu dmathieu added the ready-to-merge Code review completed; ready to merge by maintainers label Feb 26, 2025
mx-psi
mx-psi approved these changes Feb 26, 2025
View reviewed changes
@mx-psi mx-psi added this pull request to the merge queue Feb 27, 2025
Merged via the queue into open-telemetry:main with commit 4397725 Feb 27, 2025
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmathieu dmathieu dmathieu approved these changes

@bogdandrutu bogdandrutu bogdandrutu left review comments

@dehaansa dehaansa dehaansa approved these changes

@jmacd jmacd jmacd approved these changes

@mx-psi mx-psi mx-psi approved these changes

Assignees
No one assigned
Labels
processor/memorylimiter ready-to-merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[processor/memorylimiter] Add profile support for memorylimiter processor
6 participants
@iblancasa @dmathieu @dehaansa @bogdandrutu @jmacd @mx-psi